Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for custom endpoint metadata definitions #381

Merged

Conversation

robbieaverill
Copy link
Contributor

@robbieaverill robbieaverill commented Dec 6, 2021

Resolves #380

I'll make a separate PR for the docs

@shalvah
Copy link
Contributor

shalvah commented Dec 7, 2021

Very well done. But I'd rather avoid the magic ("any unknown properties go into custom"). For instance, we could add or rename a property in the future, and it would clash with something a user defined.

My idea of this was providing the custom property as a box for them to put whatever they wish in. I can see the difficulty with that, though, since returning something like this:

return [
  "groupName" => "...",
  "custom" => [
    "a" => "b"
    ]
];

would overwrite the custom array. We could deep-merge it separately, but then we'd have to be sure we capture user's intent (unsetting a property, for instance). Maybe for now, we just keep it simple: add the custom property, and allow them to manipulate it directly?

public function __invoke($endpointData)
{
  $endpointData->custom['a'] = 'b';
}

A second concern of mine is wrt serializing to the Camel files and loading from them. Did you test that the custom properties were serialized to the YAML as well, and properly loaded from them (can check this with --no-extraction)?

@robbieaverill
Copy link
Contributor Author

Sounds good, I'll update tomorrow

@robbieaverill robbieaverill force-pushed the feature/custom-endpoint-metadata branch from 983b001 to f7d767b Compare December 7, 2021 18:29
@robbieaverill
Copy link
Contributor Author

Hi @shalvah, I've updated to modify the custom property directly. I can confirm that YAML serialization and extraction is working as expected.

@robbieaverill robbieaverill force-pushed the feature/custom-endpoint-metadata branch from f7d767b to f58476e Compare December 8, 2021 01:20
public function __invoke(ExtractedEndpointData $endpointData, array $routeRules): ?array
{
$endpointData->metadata->custom['myProperty'] = 'some custom metadata';
return null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this matters, does it? Functions in PHP implicitly return null.

Oh, this is because of the type annotation requiring a return statement.

@shalvah shalvah merged commit 1285461 into knuckleswtf:master Dec 8, 2021
@shalvah
Copy link
Contributor

shalvah commented Dec 8, 2021

3.19.0 '👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom metadata strategy data not available in language templates
2 participants